-
Notifications
You must be signed in to change notification settings - Fork 3
[feat] 퀴즈 생성 기능 추가 #17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| } | ||
| } | ||
|
|
||
| private String saveThumbnail(MultipartFile file) throws IOException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[L4-변경제안]
이미지 경로를 변환하는 작업을 수행하기 때문에 "썸네일을 저장한다"는 이름보다 메서드의 동작을 잘 나타내는 이름을 정하는 것이 좋다고 생각합니다 !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
파일을 서버에 올린다(?)는 걸 저장한다고 생각해서 saveThumbnail으로 정했는데, 생각해보니 좀 모호한 메서드명인 것 같습니다. 파일을 썸네일 경로에 저장하겠다는 의미로 convertToThumbnailPath로 수정하겠습니다 !
의견 감사합니다 👍
| this.quizType = quizType; | ||
| this.thumbnailUrl = thumbnailUrl; | ||
| this.creator = creator; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[L4-변경제안]
현재 id를 제외한 모든 필드가 필수값으로 보이는데 이런 상황에선 생성자를 사용하게되면 필수값들을 강제할 수 있다는 점에서 빌더보단 생성자가 의도가 더 명확하지않나 생각됩니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
의견 감사합니다 ! 사실 처음에 생성자로 했다가 빌더로 변경했었는데, 그 이유가 아무래도 들어가는 필드값이 많다보니 생성자로 해두었을 때 가독성이 낮고, 어떤 필드에 뭐가 들어가는건지 좀 보기 힘들더라구요..! 그래서 빌더패턴으로 수정했었는데,,,
세희님 리뷰 듣고 저 두 특징에 대해 고민을 좀 해봤는데, 어차피 dto -> entity를 mapper로 분리해놨으니, 서비스에서 이 생성자 코드를 볼 일은 없다고 생각이 되고, 그렇게 되면 가독성 이슈를 조금 덜 신경써도 되겠다는 생각이 듭니다! 그래서 필드값을 강제할 수 있는 생성자로 수정하는 것도 괜찮겠다고 생각이드네요..! 이 부분 수정하겠습니다 👍
| @Transactional | ||
| public void saveQuestion(Quiz quiz, QuestionRequest request) { | ||
|
|
||
| Question question = QuestionMapper.questionRequestToQuestion(quiz, request); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[L4-변경제안]
Mapper 호출 부분은 스태틱 임포트해서 더 간결하게 표현하면 어떨까요 🤔
LimKangHyun
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다!
backend/src/main/java/io/f1/backend/domain/quiz/api/QuizController.java
Outdated
Show resolved
Hide resolved
sehee123
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고하셨습니다! 🙌
🛰️ Issue Number
🪐 작업 내용
1. 퀴즈 생성 API
퀴즈 생성 API를 생성했습니다.

2. 유의점
아직 시큐리티가 적용되어있지 않아, User는 하드 코딩해서 넣어주었습니다.
저희는 db client 콘솔으로 INSERT 문을 실행하는 게 쉽지만, 혹시나 프론트 개발자님이 그 부분이 어려우실까봐
application.yml에이 부분을 추가해두었습니다.
resources/data.sql에는 Stat이 비워져있는id=1인 User를 INSERT 해두었습니다.User creator때문에 TODO 주석이 있습니다. 참고바랍니다.3. 이미지 저장 경로
images/thumbnail/에 저장됩니다.application.yml에서 퀴즈 썸네일 디폴트 이미지 경로도 설정해두었습니다. 환경에서는 임의의 default.png를 두고 테스트를 진행했습니다.4. 확장성 관련
📚 Reference
✅ Check List